Skip to content

feat(comments): QIP comments — SIWE sign-in, VP gating, self-delete, instant post#77

Open
royalaid wants to merge 36 commits into
mainfrom
feat/qip-comments
Open

feat(comments): QIP comments — SIWE sign-in, VP gating, self-delete, instant post#77
royalaid wants to merge 36 commits into
mainfrom
feat/qip-comments

Conversation

@royalaid

@royalaid royalaid commented May 9, 2026

Copy link
Copy Markdown
Collaborator

What's in this diff

Adds the QIP comments UI to gov.mai.finance. 36 commits on top of the current main:

Comments feature:

  • useSiweSession hook — SIWE sign-in with per-address bearer token, persisted to localStorage under a versioned key so page reloads keep the composer within the session window
  • useComments hook — useInfiniteQuery comment list, optimistic self-delete mutation, and direct setQueryData cache-merge on POST so new comments appear instantly without a list refetch
  • CommentComposer — character-counted textarea with VP eligibility pre-flight (aveQi threshold shown when ineligible), sign-in CTA, and send button
  • CommentList — paginated list with sanitized markdown render, moderation menu (Hide for editors, Delete for authors, both when viewer is both)
  • SiweLoginButton — sign-in button with 90s timeout around signMessageAsync, explicit toast on rejection/dismissal, Cancel affordance during pending state
  • Graceful degradation — distinguishes "temporarily unavailable (5xx)" from "no comments yet" so the slot never shows a confusing empty state on API outages

Multi-chain Safe support:

  • Probes the Safe Transaction Service to discover which chain a Safe is deployed on, then switches the wallet to that chain before requesting the SIWE signature
  • Uses mai-api's /v2/safe-deployments endpoint as the lookup source

RPC infrastructure (landed alongside, no behaviour change for end users):

  • Per-chain viem.fallback pools with configurable rank list and observability (circuit-breaker with 30s cooldown, transport-level error metrics)
  • buildChainTransport replaces the legacy loadBalance export
  • useEnsureChain hook — pre-tx chain-switch enforcement for status mutations, Snapshot submission, and moderation writes
  • rpcUrlOverride in QCIClient gated to localhost so production builds use the curated pool

CI:

  • VITE_BASE_RPC_URLS and VITE_QIP_COMMENTS_BODY_MAX_BYTES plumbed through the GH Pages deploy workflow

Rollback

git revert <merge-sha> && git push origin main — triggers a GH Pages rebuild back to the prior state.

Post-deploy smoke (network tab)

  1. gov.mai.finance/<any-QCI> with no wallet — comments section renders, no 5xx in network tab
  2. Ineligible wallet (aveQi < 1000) — eligibility endpoint returns eligible: false, threshold copy renders
  3. Eligible wallet — SIWE sign-in completes, POST /v2/comments returns 201, comment appears instantly, persists across reload
  4. Author's own comment — "Delete" option in the menu; optimistic removal, reappears on failure

royalaid added 30 commits May 8, 2026 22:51
Adds the typed surface that the new QIP comments feature will consume.
No UI yet — this is the data layer.

src/types/comments.ts defines the public-facing types: Comment (the
listed/created row), CommentListResponse, the SIWE nonce/verify pair,
and discriminated unions for POST and moderation results. The 403
shape carries `threshold` and `currentVp` from the API so the rejection
toast can render the values the server actually saw, not anything
computed locally.

MaiAPIClient gains six methods grouped by domain:

  Auth (SIWE):
    requestQipCommentNonce(address)
    verifyQipCommentSignature({ message, signature })

  Comments:
    listQipComments({ qciId, limit?, before?, sessionToken? })
    postQipComment({ qciId, body, sessionToken? })
    hideQipComment({ commentId, reason?, sessionToken? })
    unhideQipComment({ commentId, reason?, sessionToken? })

The transport is shared via two private helpers:
  - commentsJsonRequest: throw-on-non-2xx, used by auth where every
    failure aborts the flow.
  - commentsResultRequest: returns a discriminated `{ ok, ... }` shape
    so callers can pattern-match validation errors (401, 403, 413,
    429, 503) without parsing free-form messages.

Authentication is hybrid: when sessionToken is present, send Bearer
and omit credentials. When it is absent, send credentials:include so
the same-origin HttpOnly cookie travels. Never both — a cleanly
defined precedence avoids subtle bugs where the cookie wins on one
request and the header wins on another.

The sig / sigSchemaVersion / strategiesJson columns the API stores
internally are deliberately omitted from the consumer-facing types,
matching the API's public response. They are reserved for the future
EIP-712 verifier path and must not be relied on by the frontend yet.
Drives the connect → sign → verify → authenticated state machine for the
upcoming comment composer. The hook never persists the bearer token to
localStorage / sessionStorage — it lives in component-tree React state
only, so a tab close drops the in-memory session even when the
HttpOnly cookie path isn't available.

Behavior:
  - signIn(): requestQipCommentNonce(address) → build SiweMessage with
    domain = window.location.host, uri = window.location.origin, chainId
    = 1, statement, nonce, issuedAt, expirationTime now+10min →
    signMessageAsync via wagmi → verifyQipCommentSignature(message,
    signature) → store bearer token + lowercased address in state.
  - signOut(): clears local state. The HttpOnly cookie is left in place
    (best-effort logout — there is no server-side signout endpoint yet).
  - clearOn401(): same effect as signOut, but the name flags intent at
    the call site so it's clear we're reacting to a server-side session
    expiry, not a user action.

Defensive transitions: a wallet disconnect drops the session, and an
account switch (different lowercase address) also drops the session
since the in-memory token is no longer attributable to whoever is
controlling the page.

Wallet-rejected signatures resolve with reason="user_rejected" so the
UI can stay silent (no toast — the user knows they cancelled). All
other failures resolve with reason="unknown" or "verify_failed" so the
caller can surface them.

Adds siwe@^3.0.0 to deps. The chainId is hard-coded to 1 because that
is the chain qidao.eth votes on; the actual on-chain chain the wallet
is connected to is irrelevant to a SIWE attestation, and showing
mainnet in the wallet pop-up is more familiar to voters than seeing
Base.
useComments(qciId) wraps TanStack's useInfiniteQuery against
listQipComments. Page size is 50, cursor pagination uses the API's
nextBefore field, and 30-second staleTime + refetchOnWindowFocus=false
match the existing useQCIData convention so the comment list doesn't
re-fetch every time the user tabs back to the page.

The hook also exposes a postComment mutation. Critically, it only
invalidates the comment list on result.ok — a 403 / 401 / 413 / 429 /
503 doesn't represent a new row to fold in, so the list cache is
preserved. The mutation surfaces the full discriminated PostComment
result (including the threshold and currentVp from the API on a 403)
to its caller, which is what the composer needs to render an honest
rejection toast.

useIsEditor reads hasRole(EDITOR_ROLE, address) on the QCIRegistry on
Base via the wagmi public client. The EDITOR_ROLE bytes32 is a
contract constant, so it's cached forever per registry address; the
hasRole lookup re-checks every 60 seconds, mirroring the mai-api
lib/editorRole.ts cache window so a freshly granted role takes effect
in the UI without a hard refresh. refetchOnWindowFocus is disabled to
keep RPC traffic predictable.

Both hooks return safe empty states without a connected wallet:
useComments works without auth (anonymous reads), useIsEditor returns
{ isEditor: false } until an address is present.

queryKeys gains a `qipComments(qciId)` factory and the related
editor-role keys; queryKeyPatterns.allQipComments is provided for
broad invalidation.

The frontend's UI gate via useIsEditor and the API's authoritative
hasRole check in lib/editorRole.ts are intentionally two-sided —
hiding the menu visually is not security; it is just polish so
non-editors don't see actions they can't perform.
The read-side surface for QIP comments. SafeMarkdown is the security
spine — comment bodies are user-generated markdown even when authored by
high-VP wallets, so the renderer applies four layers of defense:

  1. react-markdown 10 disables raw HTML by default. <img onerror=...>
     comes through as plain text.
  2. urlTransform restricts link/image URL protocols to http(s) and
     mailto. Anything else (javascript:, data:, ipfs:, vbscript:, ...)
     becomes a non-anchor — render as text only.
  3. The custom <a> renderer always emits target="_blank" and
     rel="noopener noreferrer nofollow", neutralising tab-nabbing and
     referrer credit even on allow-listed URLs.
  4. The custom <img> renderer returns null. Inline images would let an
     attacker beacon to arbitrary servers via redirect chains starting
     from an http(s) URL the protocol allowlist accepts; stripping is
     simpler than verifying every redirect target.

CommentList renders rows newest-first (the API already orders that way)
and uses an IntersectionObserver against a sentinel below the last row
to drive cursor pagination. The observer is only attached when there
is more data and no fetch in flight, so we don't queue duplicate page
requests. Loading state uses a small skeleton component; the empty
state is written to be readable to anyone who lands on a fresh QCI.

Each row picks up the connected wallet's editor status from
useIsEditor — non-editors don't see the moderation slot at all (the
visual gate is polish, the API enforces the actual permission).

ModerationMenu wraps the existing shadcn DropdownMenu/Dialog
primitives. The Hide flow uses TanStack's optimistic-update pattern:
onMutate removes the row from every loaded page in the cache and
captures the previous state; onError restores it; onSettled
invalidates so the API's authoritative state lands. Reasons are
optional and capped at 500 chars to mirror the API.

Author display uses ensName when present and a short 0x...abcd
fallback otherwise. Avatar is a deterministic colored circle keyed on
the address — a real avatar lib (Boring Avatars / blockies) is a
follow-up, kept out of this commit so the feature doesn't pull a new
dependency for cosmetic reasons.
Three pieces complete the write-side surface and the user-facing entry
point for the feature.

SiweLoginButton is the auth gate above the composer:
  no wallet           → ConnectKitButton (defers to the existing flow
                         configured in Web3Provider).
  wallet, no session  → "Sign in to comment" — useSiweSession.signIn()
  wallet + session    → renders nothing (composer takes over)

User-rejected wallet popups stay silent — clicking cancel is not an
error to surface. Verify failures and unknown transport errors do
toast.

CommentComposer measures the body's UTF-8 byte length via TextEncoder
so the counter agrees with the server's Buffer.byteLength check; emoji
and other multi-byte characters increment the counter by their actual
byte cost rather than UTF-16 code-unit count. The display turns amber
at 80% of the cap and red + disables the submit at 100%.

The submit handler treats every API response by status code:
  201 → clear the textarea, success toast.
  401 → clearOn401() so the SIWE prompt re-shows; keep the textarea so
        the user can re-sign and try again.
  403 → toast with the API's threshold and currentVp values
        (decimals are formatted with thousand separators), keep
        textarea.
  413 → toast with the API's reported maxBytes, keep textarea.
  429 → toast, keep textarea.
  503 → toast, keep textarea.
  *   → toast with the raw error, keep textarea.

The body byte cap is read from the new config.qipCommentsBodyMaxBytes
(VITE_QIP_COMMENTS_BODY_MAX_BYTES, default 8192). Production must set
this to whatever the API enforces so the client and server agree.

Comments<qciId/> is the top-level shell. It mounts the comment list
unconditionally (anonymous reads are allowed) and chooses between the
SIWE login or composer based on whether a token is present. The
section element carries a `key={qciId}` so navigating between QCIs
without a hard refresh resets the whole subtree cleanly — no leaked
caches between proposals.
Adds <Comments qciId={qciData.qciNumber} /> at the bottom of the
existing max-w-4xl column on the QCI detail page. The mount is
deliberately additive — no markup restructuring, no new props on the
existing components, no changes to the routing or data fetching.

The comment surface lives inside the same column as the proposal body
and the snapshot/transactions/editor controls, so it inherits the same
content width and respects the page's existing layout. It renders only
inside the main return path, which is gated by the existing
loading/error early-returns; users see the proposal body load first
and the discussion follows naturally underneath.

key={qciId} on the Comments section ensures that when the user navigates
between proposals via React Router, the comment subtree unmounts and
remounts cleanly — no leaked composer state or stale list cache from
the previous QCI.

End-to-end build verified: tsc --noEmit clean, vite build produces a
working production bundle (the chunk-size warning is pre-existing and
unrelated to this change).
Browsers reject responses to a credentialed request when the response
carries Access-Control-Allow-Origin: *, even if no actual credentials
were sent. The list endpoint is anonymous-by-design — it never needs
cookies — so the previous credentials: 'include' was both wasteful and
broken cross-origin.

Switch the GET to credentials: 'omit'. Same-origin reads still work
because anonymous reads don't depend on cookies; cross-origin reads
now succeed against the API's wildcard CORS policy.

Also lands the dev portless config so cd && portless picks up the
service name and dev script automatically.
commentsJsonRequest (auth/nonce, auth/verify) and commentsResultRequest
(comments POST, hide, unhide) used credentials: 'include' so the
HttpOnly session cookie could travel on same-origin paths. Chrome
refuses to honor responses with Access-Control-Allow-Origin: * for
credentialed requests, surfacing the failure as net::ERR_FAILED with
no useful console message — every cross-origin call was silently
broken.

Switch to credentials: 'omit' on these helpers. The Bearer token
returned from /v2/auth/qip-comments/verify is the cross-origin auth
path and stays in component-tree memory only. Same-origin deployments
that want the cookie path can land origin-specific CORS later and
flip credentials back to 'same-origin'.

Earlier the GET path was already fixed; this completes the same fix
across the auth and write paths.
useSiweSession used per-component useState, so SiweLoginButton,
the Comments shell, and CommentComposer each held their own copy of
{ status, sessionToken, address }. After a successful sign-in only
SiweLoginButton's instance held the token; the Comments shell still
saw sessionToken === undefined and continued rendering the login
button (which itself returned null because its instance had a token).
Net effect: nothing rendered between the list and the composer —
sign-in worked end-to-end on the wire but the UI dead-ended.

Replace useState with useSyncExternalStore against a module-level
singleton store (state + Set of subscribers). All consumers now read
and write the same SessionState, so a sign-in in one component
transitions every other consumer atomically. Disconnect / account
switch already cleared local state per consumer; the same logic now
clears the singleton once and notifies every subscriber.
Vite v7 ignores the PORT env var (uses --port) and binds to [::1]
(IPv6 loopback) by default. Reverse proxies like portless connect via
127.0.0.1 (IPv4 loopback), so the proxy and Vite never bridged and
every request returned 502.

Pass --host ${HOST:-127.0.0.1} and --port ${PORT:-3000} so portless
(which sets HOST and PORT) can route to Vite, while a plain `bun run
dev` still binds to localhost:3000 the way it always has.
Vercel's Vite framework preset doesn't auto-rewrite missing paths to
the SPA shell, so direct navigation to /qcis/<n> or any client-side
route returned a 404. Add a single rewrite that catches everything
except /assets/ (those should keep 404'ing if missing so build issues
are surfaced) and routes it to /index.html, letting React Router
handle the rest.
VITE_IPFS_API_URL and VITE_BASE_RPC_URL were updated on the Preview
scope in Vercel after the last build, so the existing build is still
pointing at the old (empty / localhost) values. Vite bakes env vars at
build time, so this empty commit triggers a fresh build that reads the
updated values.
Mirror Snapshot's qidao.eth chain matrix in the wagmi/ConnectKit config
so a wallet that can vote on a proposal can sign in to comments. Drop
the SIWE_CHAIN_ID = 1 hardcode in useSiweSession; embed the connected
wallet's actual chainId in the SIWE message so EIP-1271 verifiers can
look up the smart-account's contract code on the right chain.

- src/config/chains.ts: extend allowlist to mainnet, optimism, gnosis,
  polygon, base, arbitrum (+ baseSepolia in testnet). Default chain
  stays Base — non-comments routes (QCI registry reads) still want it.
- src/providers/Web3Provider.tsx: add http transports for each new
  chain with optional VITE_<NAME>_RPC_URL overrides, falling through
  to viem's chain-default public RPC. Keeps enforceSupportedChains
  on the broader allowlist.
- src/hooks/useSiweSession.ts: read useChainId() and pass through to
  the SiweMessage; Safes on Polygon now sign with chainId=137 instead
  of 1, which is what the EIP-1271 contract lookup needs.
- .env.example: document the optional VITE_*_RPC_URL overrides.
A wallet connected via Rabby's Safe import — or via ConnectKit on a
non-Safe chain — may report a chainId that doesn't match where the Safe
contract is actually deployed. EIP-1271 verification needs the message's
chainId to be the chain the smart-account code lives on, so a Safe on
Polygon signing while wagmi reports Base would fail server-side.

Add a TanStack Query hook that, on wallet connect, probes the per-chain
Safe Transaction Service across the qidao.eth allowlist (mainnet, op,
gnosis, polygon, base, arbitrum). The probe distinguishes confirmed-
not-deployed (404) from couldn't-tell (429 / network error / timeout),
so a flaky Safe TX Service falls back to the wallet's reported chainId
rather than blocking sign-in.

useSiweSession consumes the result via pickSiweChainId, which picks:
  - the single deployment when there is exactly one
  - the wallet's chainId when the Safe is deployed there
  - the first deployment otherwise
  - the wallet's chainId if the probe didn't conclude (graceful)

For EOAs the probe returns no deployments and we sign with the wallet's
chainId — chainId is informational for ECDSA recovery.
The Safe Transaction Service rejects lowercase addresses with HTTP 422
"Checksum address validation failed" (not 429 as I'd assumed). The
previous version lowercased the address before the probe, so every Safe
lookup silently came back as "unknown" — and the SIWE flow would fall
through to the wallet's reported chainId, defeating the whole point of
the deployment lookup for cross-chain Safes.

Cache by the lowercased form for stable React Query keys but pass the
original (already-checksummed by wagmi's useAccount) address into the
fetch URL. Add a 429-aware retry loop in probeOne so genuine rate-limit
hiccups absorb a 1s + 3s backoff before falling back.

Caught locally before merge by exercising the hook's helpers against
the real Safe TX Service: with the lowercase bug, both EOA and Safe
came back fully unknown; with the fix, the EOA returns no deployments
and the Safe returns chain 1. End-to-end SIWE chainId selection then
picks 8453 (wallet) for the EOA and 1 (deployment) for the Safe — the
load-bearing override.
Drop the per-chain Safe Transaction Service fan-out. The unauthenticated
Safe API tier is rate-limited at 5,000 req/month / 2 RPS, which blows
through monthly quota on a busy preview, and rejects lowercase
addresses with HTTP 422. Now hits the new mai-api endpoint which:
  - probes mainnet, polygon, base, linea via on-chain RPC
  - caches results in Redis for 90 days
  - owns retry/timeout semantics server-side

useSafeDeployments external interface is unchanged: same SafeDeployments
shape, same pickSiweChainId. Only the network call changes — useSiweSession
and SiweLoginButton see no diff.

Verified locally end-to-end against the production handler with
ethereum.publicnode.com:
  - EOA  → deployedOn=[], unknown=[]
  - Safe → deployedOn=[1], details={1:{version:"1.3.0",threshold:1,
                                       ownerCount:1}}
  - pickSiweChainId(Base, Safe) → 1 (deployment chain wins)
  - pickSiweChainId(Base, EOA)  → 8453 (falls through to wallet)
…igning

The Safe-deployment probe correctly tells us where a connecting Safe lives
(via mai-api /v2/safe-deployments), but the wallet has no awareness that
its current chain mismatches. Rabby's Safe-import flow only proposes
signatures via the Safe Transaction Service for its currently-selected
chain, so a Safe deployed on Polygon with Rabby switched to Base either
silently fails or returns a signature the verifier rejects.

useSiweSession now calls switchChainAsync before signMessageAsync when
walletChainId !== siweChainId. User rejection of the network switch is
surfaced as a new wrong_chain reason; SiweLoginButton turns that into a
"Switch your wallet to <chain> to sign in" toast naming the target chain.

Happy path is silent — the wallet pops a single switch prompt, then the
sign prompt. No UI redesign required.
Two small UX papercuts on the comment sign-in flow:

1. Rename user-facing "qipowah" copy to "aveQi". The Snapshot strategy
   identifier is technically "qipowah", but the thing it scores is what
   users recognize as aveQi from voting. The strategy ID itself is
   unchanged in network calls — only display copy.

2. Drop the second ConnectKitButton from SiweLoginButton's no-wallet
   branch. The header already owns connect UX; rendering another
   button inline within ~600px is visually redundant. Replaced with a
   short hint pointing the user at the header connect button.
Distinguish service-unavailable errors from application errors so the
comments slot doesn't render the same generic "couldn't load" copy when
the API is genuinely down (5xx, network unreachable, request timeout).

- Introduce MaiApiError in services/maiApiClient.ts with a numeric status
  property; throw it from listQipComments instead of plain Error so the
  consumer can branch on status without parsing the message text. Future
  renames of the formatted message no longer silently regress UX.
- Classify error source in CommentList: MaiApiError with status >= 500,
  AbortSignal.timeout-thrown DOMException ("TimeoutError" / "AbortError"),
  and TypeError ("fetch failed") all map to a soft retry message:
  "Comments are temporarily unavailable. Refresh to retry."
  Anything else (including 4xx like an invalid qciId) preserves the
  existing "Couldn't load comments. Try refreshing the page." copy.
- Empty-list and normal-list paths are unchanged; this only refines the
  error branch.

Verification is manual per the QIPs convention (no test runner in this
package): stub mai-api to 503 on a QCI page and confirm the soft copy
renders distinct from "no comments yet" and "Couldn't load".
Move SnapshotModerator out of the qciData.proposal !== "None" gate so editors
can recover QCIs whose status was advanced to Posted to Snapshot via the
inline status dropdown (which only writes the status, leaving snapshotProposalId
empty). Without this, the moderator UI is hidden in exactly the state where
moderators need it to call updateSnapshotProposal and link the missing ID.

The Snapshot status panel still respects the original gate -- there's nothing
to render when no proposal is linked. Only the editor recovery surface is
hoisted out.
Small wagmi v2 wrapper that components and mutation hooks can call to
guarantee the wallet is on a target chain before estimating, simulating,
or writing a transaction. Mirrors the existing useSiweSession chain-switch
pattern for SIWE signs but on the transaction side.

Returns { ensureChain, isOnChain, currentChainId, switching, targetChainId }
and throws a typed ChainSwitchRejectedError when the user declines, so
callers can show a quiet inline "Switch to <chain> to continue" hint
instead of routing every error through the toast.
- Pin publicClient to base.id so estimateContractGas / simulateContract
  never go through the wallet's RPC (the documented polygon-rpc.com 401
  path).
- Call useEnsureChain(base.id).ensureChain() before any contract reads or
  the writeContract, so the wallet is on Base before viem touches the
  network.
- Pass chain: base explicitly to writeContract for belt-and-suspenders
  assertion at the boundary.
- ChainSwitchRejectedError surfaces inline as "Switch to Base to continue"
  rather than a toast.error stack.
- Entry button reads "Switch to Base to update" while the wallet is on
  another chain, and disables while a switch is in flight.
- Pin publicClient to base.id so estimateContractGas / simulateContract
  for the linkSnapshotProposal write path always go through the Base
  transport (registered in Web3Provider), regardless of which chain the
  wallet currently reports.
- Call useEnsureChain(base.id).ensureChain() before the contract reads
  and the writeContract.
- Pass chain: base explicitly to writeContract for boundary assertion.
- ChainSwitchRejectedError surfaces a quiet inline "Switch to Base to
  link the Snapshot proposal" hint and short-circuits the outer error
  handler so it does not double-render as a destructive toast.
- Call useEnsureChain(base.id).ensureChain() at the top of the mutation
  before invoking qciClient.updateQCIStatus, so viem cannot route the
  contract reads or write through the wallet's current-chain RPC.
- Replace the dynamic createPublicClient({ chain: walletClient.chain })
  with usePublicClient({ chainId: base.id }) so waitForTransactionReceipt
  always polls Base, regardless of which chain the wallet is on after
  the write.
- Surface ChainSwitchRejectedError as a quiet toast ("Switch to Base to
  update the status") and short-circuit React Query retries on it so
  the mutation does not loop on user rejection.
…stion UX

Replace the custom round-robin loadBalance with a per-chain viem.fallback
transport stack across every chain in the wagmi config. The factory is
memoized at module scope (Map<chainId, Transport>) — load-bearing because
viem's fallback rank scheduler self-recurses forever, and the codebase has
12 QCIClient instantiation sites that would each spawn their own rank loop
without the singleton.

What's new:

- src/utils/rpcPools.ts — RPC_POOLS map keyed by chainId (Polygon defaults
  EXCLUDE polygon-rpc.com per polygon-psm-rpc-401-2026-04-22 learning),
  most-trusted-first ordering, getPoolEndpoints precedence
  (VITE_<CHAIN>_RPC_URLS comma-list > VITE_<CHAIN>_RPC_URL strict override
  > defaults — note the strict-override semantics is an intentional
  behavior change vs the prior URL+defaults composition),
  buildChainTransport(chainId, { rpcUrlOverride? }) returning a memoized
  viem.fallback. Per-http retryCount: 3 + retryDelay: 150 activates viem's
  built-in Retry-After honoring (verified at
  node_modules/viem/utils/buildRequest.ts:247-263). Outer fallback
  retryCount: 0 so failover is purely cross-endpoint. shouldThrow
  short-circuits on ContractFunctionRevertedError +
  ContractFunctionExecutionError so contract reverts propagate cleanly
  rather than being converted to RpcPoolExhaustedError.

- src/utils/rpcObservability.ts — module-level singleton tracking per
  endpoint: state ('unknown' | 'healthy' | 'cooling' | 'cors-blocked' |
  'auth-rejected'), lastFailure, demotedAt, hadOkSample. CORS detection
  uses cross-vendor regex (Chrome / Firefox / Safari) and requires
  "endpoint never had ok=true sample this session" to promote terminal —
  prevents transient blips from bricking endpoints. 403-without-Retry-After
  promotes to terminal auth-rejected state so revoked-key request storms
  don't burn quota. Auto-recovery scheduler re-probes terminal endpoints
  at 1h/6h/24h after demotedAt. window.__qipsRpc exposes getSnapshot,
  forceProbe, markEndpointHealthy, subscribe in dev.

- src/utils/RpcPoolExhaustedError.ts — typed error mirroring the
  ChainSwitchRejectedError idiom. Carries chainId + per-endpoint attempted
  metadata; outer transport in rpcPools converts viem's aggregate
  "all transports failed" shape into this when the pool exhausts.

- src/components/RpcStatusBanner.tsx — single banner with chain pills for
  multi-chain exhaustion. Retry button disables + spinner during probe;
  8s timeout flips copy to "endpoints may be unreachable from this
  network". Subscribes to pool:exhausted / pool:recovered events.

Sample buffers, in-flight counters, cumulative totals, useRpcObservability
React hook, and the visual dev panel are deferred to follow-up — v1 ships
the minimum needed by RpcPoolExhaustedError.attempted[], the banner, and
the auto-recovery scheduler.

Tighten viem pin from "2.x" to "~2.30.0" so the load-bearing source
citations (buildRequest.ts retry layer, fallback.ts rank scheduler) don't
drift across minor bumps without a deliberate review.

Plan: docs/plans/2026-05-08-003-feat-rpc-robustness-plan.md
Every chain in the wagmi transports map now flows through buildChainTransport
(the memoized per-chain pool factory from the prior commit) instead of bare
single-URL http() per chain. Mount RpcStatusBanner near the provider root so
pool-exhaustion is visible across routes. Attach window.__qipsRpc in dev for
console-level debugging.

The localBaseFork dev shim continues to use a direct http() against
config.baseRpcUrl — local Anvil flows don't need pool/observability overhead.

This consolidation into U1 (rather than deferring to U7) is the migration-
atomicity guarantee — without it, U1's pool factory would ship with no live
consumer, leaving production strictly worse than today (dead code that looks
like protection without firing).
…alance + dead ethers exports

QCIClient.ts now obtains its transport from the shared buildChainTransport
factory instead of constructing its own loadBalance round-robin. The
constructor still accepts (contractAddress, rpcUrl?, testnet?); when rpcUrl
is set, the factory honors it as a single-endpoint override (Anvil forks,
per-call routing) and bypasses the pool/observability layer. When rpcUrl
is unset, the factory returns the same memoized fallback transport used by
Web3Provider — exactly one rank loop per chain across every QCIClient
instantiation site.

Delete src/utils/loadBalance.ts entirely — no callers remain after U7
(grep -r 'loadBalance|BASE_RPC_ENDPOINTS|getRPCEndpoints' src returns 0).

Surgically remove the unused publicClientToProvider + useEthersProvider
exports from src/utils/ethers.ts. They had zero callers (verified via
grep), and the FallbackProvider branch reached into transport.transports
to instantiate ethers JsonRpcProviders directly per child URL — bypassing
the pool, the Retry-After retry layer, and onFetchResponse observability.
useEthersSigner stays — SnapshotSubmitter still needs the wallet-client →
ethers.Signer bridge for snapshot.js's signing path.

Plan: docs/plans/2026-05-08-003-feat-rpc-robustness-plan.md
…ically-bad endpoints

Source: DefiLlama/chainlist's extraRpcs.js, applied filters:
- https only (wss deferred per the plan)
- no embedded API keys (/<32-hex-chars>/ patterns, ?api_key=)
- no tracking: "yes" providers (Tenderly, Tatum, Lava, Numa, BloxRoute)
- no Alchemy/Infura /v2/demo URLs (rate-limit immediately)
- drop URLs observed failing browser CORS preflight from
  qips.qidao.localhost (1rpc.io/op, 1rpc.io/arb)
- drop URLs that consistently cool on first probe across all chains
  ((*.therpc.io and *.llamarpc.com both rate-limit aggressively per
  smoke against the live qips dev server))

Counts per chain after curation:
  Base:         7 endpoints
  Base Sepolia: 3
  Mainnet:      7
  Polygon:      4 (polygon-rpc.com still excluded per polygon-rpc-401 learning)
  Optimism:     5
  Arbitrum:     7
  Gnosis:       5

Smoke verified via window.__qipsRpc.forceProbe(chainId) on the live dev
server: every retained endpoint reaches `healthy` state on first probe.
The viem.fallback rank handles the rest at runtime — partial degradation
during a vendor-side incident demotes the affected endpoint and the next
healthy one in the input order takes over.

Doc the chainlist refresh procedure in the file header.
…Ls use the pool

The U7 migration passed the QCIClient constructor's existing rpcUrl arg
straight into buildChainTransport's rpcUrlOverride. Every QCIClient
instantiation site (12 of them) passes config.baseRpcUrl, which
resolves to https://mainnet.base.org from .env. The override path
returns a single-endpoint http() transport — collapsing the new
6-endpoint Base pool back to one URL and triggering 429 storms on
hard reload of the QCI list page.

Web3Provider's wagmi-routed reads were unaffected (they call
buildChainTransport(base.id) with no override), so smoke-verify
during plan execution showed wagmi's path working and missed the
QCIClient direct read path entirely. The codex iter-3 plan-review
F2 finding flagged exactly this risk; the plan-time response threaded
rpcUrl||undefined into rpcUrlOverride and produced the inverse bug.

Fix: rpcUrlOverride is now gated on isLocal (localhost / 127.0.0.1
detection) so it only fires for Anvil-fork dev. Production URLs
ignore the override and fall through to the per-chain pool — the
same memoized fallback transport that Web3Provider uses.

Verified in browser: Base pool now reports 5 healthy endpoints and 1
cooling (the user's onfinality override URL, rate-limited as expected),
QCI detail page renders successfully, no 429 storm.

Compounded as docs/solutions/integration-issues/qciclient-rpc-override-bypassed-pool-2026-05-08.md
royalaid added 6 commits May 8, 2026 22:55
…he composer

The bearer token from /v2/auth/qip-comments/verify lived only in module-scoped
memory, while the HttpOnly Set-Cookie is unusable cross-origin (mai-api and the
QIPs frontend are different origins, and the cookie is SameSite=Strict in prod
which blocks cross-origin sends entirely). On reload the in-memory token was
gone, the cookie wasn't read, and the user landed back on the sign-in button
even though their server-side session was still valid.

Persist {token, address, expiresAt} under a versioned localStorage key
(qip-comments:siwe-session:v1), hydrate the module-scoped singleton once at
module init before any component mounts, and clear on sign-out, 401, wallet
switch, expiry, or actively-disconnected state.

Switch the disconnect-detection from !isConnected to status === 'disconnected'
so wagmi's transient 'connecting' / 'reconnecting' window during reload
doesn't discard the persisted session before the connector has a chance to
hydrate.
…ejection

If the user clicked "Sign in to comment", the wallet popup appeared, and the
user dismissed it without signing or rejecting, signMessageAsync could hang
indefinitely on some wallets (notably Rabby's Safe-import flow). The button
stayed disabled on "Signing in…" with no recovery affordance — the box
appeared to "disappear" from the user's perspective.

Layer three guards that together make the rejection path always recoverable:

  1. 90s Promise.race timeout around signMessageAsync; on timeout reset
     state to 'idle' and synthesise a user_rejected reason.
  2. finally block at the top of signIn that asserts status !== 'signing'
     before returning. Belt-and-suspenders against future regressions
     forgetting one of the explicit setSessionState resets.
  3. SiweLoginButton replaces the silent return on user_rejected with an
     info toast naming the recovery affordance: "Sign-in cancelled. Click
     'Sign in to comment' to try again." Covers both explicit rejects and
     timeouts, since the timeout path collapses to user_rejected.

Other rejection branches (wrong_chain, verify_failed, unknown) keep their
existing toasts — regression-checked by re-reading the rendered branches.
…seComments

Two coupled changes to the comments hook:

1. POST cache merge. POST /v2/comments already returns the inserted
   comment with ENS resolved server-side. The hook was throwing that
   row away and calling queryClient.invalidateQueries, which fires a
   full refetch of the infinite-query pages — visible to the user as
   a delay between the toast firing and the comment showing up. Slow
   connections read this as "the post failed, refresh the page".
   Replace with setQueryData that prepends the returned row to
   pages[0].comments. staleTime: 30s remains so the merged row isn't
   overwritten by a refetch the user didn't ask for.

2. Optimistic self-delete mutation. New deleteComment(args) on
   UseCommentsResult: onMutate snapshots the cache and filters the
   row out of every page so the UI feels instant; onError restores
   the snapshot on rejection. The wrapper preserves the discriminated-
   union return shape so callers can branch on result.status (401/
   403/404/409/etc.) and render specific toasts.

Subsequent background refetches dedupe naturally by id; the optimistic
removal already matches server truth on success.
Extend ModerationMenu with an owner mode that lives alongside the existing
editor mode:

- Editor takes precedence (an editor reading their own comment uses Hide,
  not Delete — mirrors how every comment platform handles overlap).
- Owner mode renders only when viewerAddress matches comment.author
  (case-insensitive) AND the comment is currently visible. Hidden
  comments offer no Delete trigger to the author since they're already
  removed from public view.
- Confirmation dialog before delete; can't-be-undone copy.
- Wires deleteComment from useComments — optimistic removal with rollback
  on rejection.
- Toast routing for 401 (session expired -> clearOn401), 403 (defensive,
  UI shouldn't expose this state), 404, 409, and other failures.

Plumbing:

- DeleteOwnCommentRequest/Response types in comments.ts. Response is the
  same shape as ModerationResponse so callers can pattern-match the same
  union members; the new error code 'not_owner' fits the 403 slot.
- maiApiClient.deleteOwnQipComment posts to /v2/comments/{id}/delete
  with no body (the server forces hiddenReason='self_deleted', so a
  malicious client cannot spoof an editor reason).
- CommentList passes viewerAddress + isEditor + commentAuthor down so the
  menu can compute its own mode and render only when there's a meaningful
  action available.
…or and author

The original split treated editor/owner as mutually exclusive modes, with
editor taking precedence — meaning an editor reading their own visible
comment got Hide but no Delete. That hides a legitimate self-action
behind editor capability instead of offering both, and surprised editors
trying to delete their own posts.

Render each applicable item independently inside the same dropdown:

  isEditor && !isHidden  → "Hide comment"      (reversible, audit-tagged)
  isEditor && isHidden   → "Restore comment"
  canDeleteOwn           → "Delete"            (irreversible self-removal)

canDeleteOwn requires viewerAddress (lowercased) to match comment.author
and the comment to be currently visible. Server still enforces each
action's authorization independently of these UI gates.
The build needs the multi-RPC pool list and the body byte cap as
build-time env vars. Both go through the standard unprefixed-secret →
VITE_-prefixed-env mapping convention used elsewhere in this workflow.

VITE_BASE_RPC_URLS feeds buildChainTransport's viem.fallback rank list
for Base; an unset or empty value falls through to the chainlist-curated
defaults in src/utils/rpcPools.ts. VITE_QIP_COMMENTS_BODY_MAX_BYTES
defaults to 8192 (8 KB) to match the API server's matching default;
client + server caps must agree so they reject the same inputs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant